Minor update#67
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughAdded Changes
Sequence Diagram(s)sequenceDiagram
participant UI
participant AppController
participant EditorBackend
participant PreviewWorker
UI->>AppController: enter crop mode (toggle_crop_mode)
AppController->>AppController: _reset_crop_only()
AppController->>EditorBackend: clear currentCropBox / reset straighten_angle (preserve rotation)
AppController->>UI: ui_state.isCropping = True
AppController->>UI: ui_refresh_generation += 1
AppController->>PreviewWorker: kick preview worker for updated crop state
UI->>AppController: cancel crop mode (cancel_crop_mode)
AppController->>AppController: _reset_crop_only()
AppController->>EditorBackend: clear currentCropBox / reset straighten_angle (preserve rotation)
AppController->>UI: ui_state.isCropping = False
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@faststack/app.py`:
- Line 6847: _current behavior_: _reset_crop_settings() also clears rotation,
which causes rotation edits to be lost when toggling/canceling crop mode. Fix by
splitting responsibilities: add a new method (e.g., _reset_crop_mode() or
_reset_crop_only()) that clears only crop-specific state (crop box, aspect,
flip/scale if crop-specific) without touching rotation, and leave
_reset_crop_settings() (or create _reset_all_transform_settings()) as the full
reset that continues to clear rotation. Replace calls that run when
entering/exiting/canceling crop mode (the current callers of
_reset_crop_settings()) to call the new _reset_crop_only() so rotation is
preserved; keep any callers that truly need to clear rotation pointed at the
full-reset method. Ensure tests or UI flows that expect rotation to persist
after crop toggle now use the new crop-only reset.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 30e8261213
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # Reset to full image defaults (UI and Backend) | ||
| self.ui_state.currentCropBox = (0, 0, 1000, 1000) | ||
| self.image_editor.set_crop_box(None) | ||
| self._reset_crop_settings() |
There was a problem hiding this comment.
Calling _reset_crop_settings() here resets image_editor's rotation to 0, so entering crop mode now clears any previously applied 90°/180° rotation edits. In practice, if a user rotates an image and then toggles crop mode, the orientation is silently lost before cropping or canceling, which is a behavior regression from the previous code path that only cleared straighten_angle and crop selection.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
faststack/app.py (1)
6851-6855:⚠️ Potential issue | 🟠 MajorCancel no longer discards crop straighten changes.
cancel_crop_mode()says it exits without applying changes, but after this switch it only resets the crop box. Anystraighten_angleset during the crop session is left incurrent_edits, so the preview stays straightened and a later save can persist an edit the user explicitly cancelled.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@faststack/app.py` around lines 6851 - 6855, The cancel_crop_mode flow currently only calls _reset_crop_only() and leaves any straighten_angle in current_edits, so cancelled straighten changes persist; update cancel_crop_mode (or extend _reset_crop_only) to explicitly remove or reset current_edits['straighten_angle'] (and any related preview state) before incrementing ui_refresh_generation and calling _kick_preview_worker(), ensuring the preview is reverted to the pre-crop orientation and that a later save cannot persist the cancelled straighten; reference cancel_crop_mode, _reset_crop_only, current_edits, straighten_angle, _kick_preview_worker, ui_refresh_generation, and update_status_message when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@faststack/app.py`:
- Around line 6879-6883: The current code sets self.ui_state.isCropping = True,
then calls self._reset_crop_only() which clears isCropping, then re-sets it —
causing an unwanted exit+re-enter transition; fix by calling
self._reset_crop_only() first and then set self.ui_state.isCropping = True
(remove the initial pre-reset assignment) so crop mode is enabled only once and
avoids the false→true bounce.
---
Outside diff comments:
In `@faststack/app.py`:
- Around line 6851-6855: The cancel_crop_mode flow currently only calls
_reset_crop_only() and leaves any straighten_angle in current_edits, so
cancelled straighten changes persist; update cancel_crop_mode (or extend
_reset_crop_only) to explicitly remove or reset
current_edits['straighten_angle'] (and any related preview state) before
incrementing ui_refresh_generation and calling _kick_preview_worker(), ensuring
the preview is reverted to the pre-crop orientation and that a later save cannot
persist the cancelled straighten; reference cancel_crop_mode, _reset_crop_only,
current_edits, straighten_angle, _kick_preview_worker, ui_refresh_generation,
and update_status_message when making the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
1 participant
Summary by CodeRabbit
New Features
Refactor